Conversation
cielbellerose
requested review from
Steph375,
chpy04 and
wavehassman
and removed request for
Steph375,
chpy04 and
wavehassman
wavehassman
left a comment
wavehassman
left a comment
There was a problem hiding this comment.
Great job Kinsey! I'm fine with taking subtotal out of the backend endpoint and calculating it in the service function, but if we're going to do that, I want to take it out everywhere in the frontend and other places. If someone were to go back later, it would be pretty confusing to have it in some places but not others.
| quantity, | ||
| unitName, | ||
| price, | ||
| subtotal, |
There was a problem hiding this comment.
you have to take subtotal out of the route too
| manufacturer = await BillOfMaterialsService.getSingleManufacturerWithQueryArgs(manufacturerName, organization); | ||
| } | ||
|
|
||
| // recalculate subtotal on edits |
There was a problem hiding this comment.
if we're going to take subtotal out of the endpoints in the backend, we should take it out of the frontend hooks too since now it's being passed and then dropped
|
|
||
| const processRowUpdate = async (newRow: BomRow, oldRow: BomRow): Promise<BomRow> => { | ||
| // assemblies are not editable | ||
| if (String(newRow.id).startsWith('assembly')) return newRow; |
There was a problem hiding this comment.
id is already a string in the BomRow type so casting it is unecessary
| renderCell: renderStatusBOM, | ||
| renderCell: (params) => { | ||
| // assemblies are not editable | ||
| if (!params.value || String(params.row.id).startsWith('assembly')) return null; |
2 participants
Changes
BOM inline editing
Notes
Removed the 'None' special handling for PDM file name to make inline editing more intuitive (None vs empty it was just a bit odd to handle inline)
Test Cases
Screenshots
Checklist
It can be helpful to check the
ChecksandFiles changedtabs.Please review the contributor guide and reach out to your Tech Lead if anything is unclear.
Please request reviewers and ping on slack only after you've gone through this whole checklist.
yarn.lockchanges (unless dependencies have changed)Closes #4107